-
Notifications
You must be signed in to change notification settings - Fork 6
Feature/vplay 11969 text lang bug fix #734
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev_sprint_25_2
Are you sure you want to change the base?
Conversation
If the text track is changed without a re-tune then priv_aamps mPreferredTextTrack, and stream abstractions mTextTrackIndex are not updated to reflect the current track The manifest url can be set to null due to an issue with passing the same string into cache handler methods as multiple params by reference.
If the text track is changed without a re-tune then priv_aamps mPreferredTextTrack, and stream abstractions mTextTrackIndex are not updated to reflect the current track The manifest url can be set to null due to an issue with passing the same string into cache handler methods as multiple params by reference. The specified language via SetTextTrack() is ignored in perference to the prefferedTextLanguage list.
If the text track is changed without a re-tune then priv_aamps mPreferredTextTrack, and stream abstractions mTextTrackIndex are not updated to reflect the current track The manifest url can be set to null due to an issue with passing the same string into cache handler methods as multiple params by reference. The specified language set via SetTextTrack() is ignored in perference to the prefferedTextLanguage list.
4c64a85 to
6c40988
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR addresses three related issues with text track handling in AAMP: stale text track information, incorrect text language preference priority, and manifest URL corruption in the cache handler.
Key Changes
- Cache Handler Fix: Modified
RetrieveFromPlaylistCacheandRetrieveFromInitFragmentCacheto pass URL by value instead of const reference, preventing corruption when the same string is used for bothurlandeffectiveUrlparameters - Text Track Selection Logic: Added check to respect pre-set preferred text tracks instead of always overriding with language preferences during stream initialization
- State Consistency: Added calls to
SetPreferredTextTrackandSetCurrentTextTrackIndexto maintain consistent state when text language changes don't trigger a retune
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| AampCacheHandler.h | Changed RetrieveFromPlaylistCache and RetrieveFromInitFragmentCache signatures to pass URL by value |
| AampCacheHandler.cpp | Implemented URL pass-by-value fix and simplified conditional logic for effective URL assignment |
| FakeAampCacheHandler.cpp | Updated fake implementation signatures to match new interface |
| StreamAbstractionAAMP.h | Added SetCurrentTextTrackIndex method to enable text track index updates without retune |
| fragmentcollector_hls.cpp | Modified text track selection to only auto-select from language preferences if no preferred track is already set |
| priv_aamp.cpp | Added calls to update preferred text track and current text track index when language changes without retune |
If the text track is changed without a re-tune then priv_aamps mPreferredTextTrack, and stream abstractions mTextTrackIndex are not updated to reflect the current track The manifest url can be set to null due to an issue with passing the same string into cache handler methods as multiple params by reference. The specified language set via SetTextTrack() is ignored in perference to the prefferedTextLanguage list.
VPLAY-11969: GetTextTrackInfo returns stale information
Fixes the following:
Stale information being returned for GetTextTrackInfo when no tune happens on track change
Using the preferred text language setting over the user setting when selecting the text track to use
Corruption of the manifest URL when it is passed into the cache by reference for both the URL and the effective URL
Risk: Low